Adopt typed params for pipeline-specific parameters - #77
Merged
Conversation
Fixes boolean options (e.g. --save_domtblout false) not being turnable off from the command line, since untyped params take the truthy string "false" rather than a coerced Boolean. Params read directly within nextflow.config itself (outdir, custom_config_base, trace_report_suffix, monochrome_logs, etc.) can't use this syntax -- it's script-only -- and stay declared there. nf-core/tools' nextflow_config and schema_params lint checks don't yet know about typed params and hard-fail against them, so those two checks are disabled in .nf-core.yml, matching nf-core/ampliseq's own precedent for this transition. Raises the minimum Nextflow version to 26.04.0 (required for typed params) and bumps nf-schema to 2.8.0.
|
Closed
erikrikarddaniel
marked this pull request as ready for review
August 27, 2026 10:41
Joon-Klaps
reviewed
Aug 28, 2026
Joon-Klaps
left a comment
Joon-Klaps
left a comment
There was a problem hiding this comment.
Don't want to approve yet as I'm not certain how critical the things are that I pinpointed.
There was a problem hiding this comment.
Was hesitant to review due to lack of experience on this. But given you posted twice.
Shouldn't these String? need to be Path? ?
Member
Author
There was a problem hiding this comment.
Tested it against real Nextflow (26.04.6): a Path?-typed param given an HTTPS URL fails with Input file '...' does not exist -- Nextflow resolves it as a local filesystem path rather than a remote URL. Since phyloplace's own test profiles pass file params as HTTPS URLs, Path? would break them, so String? is intentional here.
Comment on lines
15
to
16
| outdir = null | ||
| publish_dir_mode = 'copy' |
There was a problem hiding this comment.
You can't add typed inputs here and in the other remaining ones of params scope in the config?
Member
Author
There was a problem hiding this comment.
Good push -- made us look again at what was actually still there and why.
Verified: a typed params { } block still hard-fails inside a .config file on current Nextflow (Unexpected input: '='), it's script-only syntax. That's why outdir, publish_dir_mode, custom_config_base/custom_config_version and trace_report_suffix stay here: they're all read directly by nextflow.config itself (or by conf/modules.config, included from it) at config-parse time, before main.nf -- where the typed block lives -- is even parsed. No restructuring moves those out. help stays for a separate reason: a boolean-or-string schema type with no single matching Nextflow type.
But your comment did make us re-check whether everything remaining here actually needed to stay, and it turned up a real one: monochrome_logs is a genuine Boolean that, apart from validation.monochromeLogs, isn't read at config-parse time at all, so it didn't need to be here. Moved it into the typed block (ff6966f), which fixes the exact same --foo false coercion bug for it too -- confirmed --monochrome_logs false was still evaluating truthy before that. Nothing else left in this params block is a Boolean, so there's nothing further to move.
Only nextflow.config's own validation.monochromeLogs assignment needs it at config-parse time (undeclared params.* reads still work there); every script-side use (main.nf's take: args, and the boilerplate subworkflow's `if (monochrome_logs)` check) now gets a real coerced Boolean, fixing the same CLI-string-coercion bug for this param that PR nf-core#77 already fixed for the others. Nothing else left in nextflow.config's untyped params block is a Boolean, per review on nf-core#77. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dpvh3T7aVgU5XyjqcYQLQD
Joon-Klaps
approved these changes
Aug 28, 2026
Member
Author
|
Thanks @Joon-Klaps ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR checklist
nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).Description
Closes #74.
Adopts Nextflow's typed
params {}block for phyloplace's own pipeline-specific parameters, fixing the underlying bug: a boolean option can't be turned off from the command line with untyped params, since--save_domtblout falsearrives as the truthy string'false', not a coercedBoolean.A few things worth knowing:
params {}blocks are script-only syntax -- they can't live innextflow.config. Params thatnextflow.configitself reads directly at config-parse time (outdir,custom_config_base/custom_config_version,trace_report_suffix,monochrome_logs) stay declared there as before; everything else moved into a typed block at the top ofmain.nf.helpalso stays untyped, since its schema type is aboolean|stringunion (bare--helpvs.--help <topic>) that doesn't map to a single Nextflow type.nextflow_configandschema_paramslint checks don't know about typed params yet and hard-fail against any schema param not also assigned innextflow.config. Disabled both in.nf-core.yml, matching the same approach nf-core/ampliseq already uses on itsdevbranch for this exact transition (with the sameTODO: Remove when tools supports parameter typesnote) -- their equivalent lint CI is green with this config, and there's no nf-core/tools issue yet tracking support for this.